Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pre-commit & black #1340

Merged
merged 2 commits into from
Aug 19, 2022
Merged

pre-commit & black #1340

merged 2 commits into from
Aug 19, 2022

Conversation

ulgens
Copy link
Collaborator

@ulgens ulgens commented Aug 18, 2022

  • Moves black into the pre-commit wrapper
  • Applies black to all files in the repo

@ulgens ulgens requested a review from erikwrede August 18, 2022 11:32
@ulgens ulgens changed the base branch from main to v2 August 18, 2022 11:33
@ulgens ulgens self-assigned this Aug 18, 2022
@ulgens ulgens added the v2 label Aug 18, 2022
@ulgens
Copy link
Collaborator Author

ulgens commented Aug 18, 2022

@erikwrede @nikolaik 👋🏻 🌹

erikwrede
erikwrede approved these changes Aug 18, 2022
View reviewed changes
Copy link
Member

@erikwrede erikwrede left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, there can't be much wrong with black formatting!

But again, we should aim to unify or configs to have some consistency over all the projects:
https://github.com/graphql-python/graphene/blob/master/.pre-commit-config.yaml

  • why do we still have a makefile in this project?

@ulgens
Copy link
Collaborator Author

ulgens commented Aug 19, 2022

we should aim to unify or configs to have some consistency over all the projects:

I agree. I will create another PR to update hooks in graphene repo. And in this repo, we can re-order hooks to match ordering in the graphene repo.

why do we still have a makefile in this project?

Probably because nobody removed it yet. It doesn't break anything, so IMO it's low priority.

@ulgens ulgens merged commit 8383bdc into v2 Aug 19, 2022
@ulgens ulgens deleted the precommit-black branch August 19, 2022 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erikwrede erikwrede erikwrede approved these changes

Assignees

@ulgens ulgens

Labels
v2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ulgens @erikwrede